Skip to content

[fix] fit to bounds - fix initial basemap and deck projections mismatch - #3155

Merged
igorDykhta merged 2 commits into
masterfrom
igr/bounds-max-out-bug-test
Jul 10, 2025
Merged

[fix] fit to bounds - fix initial basemap and deck projections mismatch#3155
igorDykhta merged 2 commits into
masterfrom
igr/bounds-max-out-bug-test

Conversation

@igorDykhta

@igorDykhta igorDykhta commented Jul 9, 2025

Copy link
Copy Markdown
Collaborator

Before this fix, the initial zoom levels for the basemap and Deck layers could differ, requiring user input to synchronize the projections. This issue affected any layer whose bounds included latitude values closer the poles.

  • getCenterAndZoomFromBounds - add extra zoom logic from normalizeViewportProps
  • width was used instead of height

Example how dataset looked right after loading before the fix:
bounds mis match

Screenshot 2025-07-10 at 3 43 05 PM

After the fix:
Screenshot 2025-07-09 at 4 10 47 AM

Signed-off-by: Ihor Dykhta <dikhta.igor@gmail.com>
Copilot AI review requested due to automatic review settings July 9, 2025 01:06

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR synchronizes initial zoom between the basemap and Deck layers by enforcing a minimum zoom based on viewport height and corrects a typo in the fitBounds call.

  • Added allowFloat flag and enforced minZoom logic in getCenterAndZoomFromBounds to match Deck’s normalizeViewportProps
  • Corrected height parameter in the fitBounds call (was incorrectly using width)
  • Updated geoViewport.viewport invocation to use a let-bound zoom for mutation

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/utils/src/projection-utils.ts Added minZoom enforcement and default center adjustment logic
src/reducers/src/map-state-updaters.ts Fixed typo: use height instead of width for fitBounds call
Comments suppressed due to low confidence (2)

src/utils/src/projection-utils.ts:62

  • [nitpick] Add a comment explaining why the center latitude is reset to zero when zoomminZoom, so future readers understand the expected behavior and its relation to Deck’s normalization logic.
    center[1] = 0;

src/utils/src/projection-utils.ts:59

  • Consider adding unit tests for cases where the computed zoom is below minZoom to verify that the enforced minimum zoom and center adjustment behave as intended.
  const minZoom = Math.log2(height / MAPBOX_TILE_SIZE);

Comment thread src/utils/src/projection-utils.ts
Signed-off-by: Ihor Dykhta <dikhta.igor@gmail.com>
@igorDykhta
igorDykhta requested a review from ilyabo July 9, 2025 17:56
@igorDykhta
igorDykhta merged commit 0473347 into master Jul 10, 2025
8 checks passed
@igorDykhta
igorDykhta deleted the igr/bounds-max-out-bug-test branch July 10, 2025 13:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants